feat: Add azure attributes in federated database instance resource #3484
Conversation
…_database_instance
test: add Azure cloud provider acceptance test for federated database resource
…est functionality on Azure Cloud Provider Missing refactor
| Type: schema.TypeList, | ||
| MaxItems: 1, | ||
| Required: true, | ||
| Optional: true, |
There was a problem hiding this comment.
Investigation notes:
cloud_provider_config(parent attribute) is not required in the create operation and is not returned from the API when it is not set. However, the attribute is already marked as Optional+Computed, so keeping it as is to allow no-op when the attribute is removed.awschanging fromrequiredtooptionalto allowazureusage
There was a problem hiding this comment.
Addressed in d7da829ac0db2f95da04417bce4fb47c9c5eb1c6
There was a problem hiding this comment.
This information: Changing from required to optional to allow azure usage I would keep only as a PR comment.
Later after this is merged it will be no surprise to find this as optional.
internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go
Outdated
Show resolved
Hide resolved
internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go
Outdated
Show resolved
Hide resolved
internal/service/federateddatabaseinstance/resource_federated_database_instance.go
Outdated
Show resolved
Hide resolved
internal/service/federateddatabaseinstance/resource_federated_database_instance.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
We need to add:
AZURE_ATLAS_APP_ID: ${{ inputs.azure_atlas_app_id }}
AZURE_SERVICE_PRINCIPAL_ID: ${{ inputs.azure_service_principal_id }}
AZURE_TENANT_ID: ${{ inputs.azure_tenant_id }}
Here: MONGODB_ATLAS_FEDERATION_SETTINGS_ID: ${{ inputs.mongodb_atlas_federation_settings_id }} so the test can use Azure in CI
There was a problem hiding this comment.
Looks like we need to update the schema of the data sources too. Due to this failing tests:
https://github.com/mongodb/terraform-provider-mongodbatlas/actions/runs/16174869633/job/45657910272#step:5:91
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*ResourceData).Set(0xc001192750, {0x2d7628f, 0x15}, {0x2619600, 0xc001a8eff0})
/home/runner/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/resource_data.go:238 +0x2b2
github.com/mongodb/terraform-provider-mongodbatlas/internal/service/federateddatabaseinstance.dataSourceMongoDBAtlasFederatedDatabaseInstanceRead({0x3197340, 0xc00057e150}, 0xc001192750, {0x2864b80?, 0xc001b6d740?})
/home/runner/work/terraform-provider-mongodbatlas/terraform-provider-mongodbatlas/internal/service/federateddatabaseinstance/data_source_federated_database_instance.go:336 +0x78d
internal/service/federateddatabaseinstance/data_source_federated_database_instance.go
Outdated
Show resolved
Hide resolved
internal/service/federateddatabaseinstance/resource_federated_database_instance.go
Outdated
Show resolved
Hide resolved
…o its own function to be able to reuse it in multiple places
|
This PR has gone 7 days without any activity and meets the project’s definition of "stale". This will be auto-closed if there is no new activity over the next 7 days. If the issue is still relevant and active, you can simply comment with a "bump" to keep it open, or add the label "not_stale". Thanks for keeping our repository healthy! |
|
Test failing due to config issue, which is also happening on nightly run tests. |
|
APIx bot: a message has been sent to Docs Slack channel |
| cloudProviderDataSource = ` | ||
| data "mongodbatlas_cloud_provider_access_setup" "test" { | ||
| project_id = mongodbatlas_cloud_provider_access_setup.test.project_id | ||
| provider_name = "AWS" |
There was a problem hiding this comment.
AWS provider name for the Azure config?
There was a problem hiding this comment.
Addressed in 868111fc337ccae273e8aa4fcde771584be2e189
| Required: required, | ||
| Computed: computed, | ||
| }, |
There was a problem hiding this comment.
| Required: required, | |
| Computed: computed, | |
| }, | |
| Required: !isDataSource, | |
| Computed: isDataSource, | |
| }, |
nit: Having the required/computed variables adds one additional level of indirection, might be easier to follow if the criteria is explicit in the schema directly.
There was a problem hiding this comment.
I understand the concern about the level of indirection, however I believe for readability purposes it's better to keep the variables as they are, since it won't require a mental mapping of the boolean logic.
| var maxItems int | ||
| if isDataSource { | ||
| computed = true | ||
| maxItems = 0 |
There was a problem hiding this comment.
any reason why we would set max items to 0?
If we go for modifying the data source schema of existing attributes Ideally aws would simply be Computed-only attribute. Since we have this inconsistent schema in the data source as for the scope of this PR would leave it as is, similar to test_s3_bucket.
If done as part of this ticket we have to ensure it has no impact and I would go for setting as compute-only over modifying the max items.
There was a problem hiding this comment.
From what I understand, setting MaxItems = 0 is basically the same as leaving it unset. Since this function is used for both resource and data source schemas, setting it to 0 won't have any impact. Only for the resource, where it's set to 1.
marcabreracast
deleted the
CLOUDP-245406_DataFederation_AddAzureSupportForCloudProviderConfig
branch
…_peering * CLOUDP-320243-dev-2.0.0: fix: Changes `actions` attribute to TypeSet in `mongodbatlas_custom_db_role` to not be sensitive to order (#3508) chore: Updates CHANGELOG.md for #3513 fix: Sets org_id on import of `mongodbatlas_organization` resource (#3513) go 1.24.5 (#3514) chore: Updates CHANGELOG.md for #3484 feat: Add azure attributes in federated database instance resource (#3484) chore: Updates CHANGELOG.md for #3505 feat: Add support for `MONGODB_ATLAS_PUBLIC_API_KEY` and `MONGODB_ATLAS_PRIVATE_API_KEY` to TF (#3505)
Description
federated_database_instanceacc.cloud_provider_access.goLink to any related issue(s): CLOUDP-245406
Follow up Work
Type of change:
Required Checklist: